Skip to content

chore: backport 20260527#1317

Merged
weng271190436 merged 5 commits into
Azure:mainfrom
weng271190436:weiweng/backport20260527
May 28, 2026
Merged

chore: backport 20260527#1317
weng271190436 merged 5 commits into
Azure:mainfrom
weng271190436:weiweng/backport20260527

Conversation

@weng271190436
Copy link
Copy Markdown
Member

Description of your changes

515a744 (cncf/main, cncf/HEAD) feat: migrating to validating admission policies for our validating logic (1/) (#708)
28401ae feat: allow PickFixed placement with empty ClusterNames for scale-to-zero (#719)
803d306 feat: harden property selector validation (#668)
99653bd chore: bump step-security/harden-runner from 2.19.1 to 2.19.2 (#710)

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

dependabot Bot and others added 5 commits May 16, 2026 15:20
)

Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.19.1 to 2.19.2.
- [Release notes](https://github.com/step-security/harden-runner/releases)
- [Commits](step-security/harden-runner@a5ad31d...9ca718d)

---
updated-dependencies:
- dependency-name: step-security/harden-runner
  dependency-version: 2.19.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: harden property selector validation

Address the four validator-hardening TODOs from issue Azure#646. Two were
stale (the placement validator already enforced them at the API
boundary); two are real and now have implementations + thorough tests.

What's new:

- Bundle operator and value validation into validateOperatorAndValues
  driven by a single supportedPropertyOperators table. Replaces the
  prior split validateOperator / validateValues. Preserves the
  existing "operator X requires exactly one value, got N" wording so
  existing tests stay stable, and only emits "exactly N values" if a
  future operator declares requiredValueCount > 1.

- Add per-property logical-contradiction detection
  (validateRequirementsConsistency). Split into named helpers per
  reviewer feedback: collectRequirementBounds, tightenLower /
  tightenUpper (with a strict-preferred tiebreak), checkEqVsNe,
  checkInterval, checkEqInsideBounds. Detects:
    * two Eq with different values,
    * Eq + Ne on the same value,
    * empty intervals from the most-restrictive lower vs the
      most-restrictive upper, including boundary cases such as
      Gt x + Lt x, Gt x + Lte x, Gte x + Lt x,
    * Eq value violating the most-restrictive lower or upper bound.
  More elaborate cases (sparse Ne exclusion sets, etc.) are left for
  the property-selector evaluator at scheduling time.

What's gone:

- Removed two stale TODOs in resource_selector_resolver.go (lines 526,
  556). Mutual-exclusiveness of Name vs LabelSelector and labelSelector
  validity are already enforced in pkg/utils/validator/placement.go
  (validatePlacement / validateLabelSelector). Replaced with comments
  pointing readers at the validator.

Tests:

- TestValidateOperatorAndValues (7 cases including unsupported
  operator, malformed quantity, count mismatch).
- TestValidateRequirementsConsistency (~20 cases: each contradiction
  variant plus negative controls and malformed-input skipping).
- TestRequirementBoundsTighten (4 cases for the "most restrictive
  wins" + tie-breaks).

Boy Scout: switched two interface{} to any in the Service nodePort
stripping block.

Fixes Azure#646

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>

* Address feedback

Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>

* Address feedback

Consolidate the per-operator bounds-folding behaviour into the
supportedPropertyOperators registry so the spec table is the single
source of truth for operator validity, required value count, and how
each requirement folds into requirementBounds. collectRequirementBounds
no longer carries a switch; it dispatches via spec.applyToBounds.

- Add operatorSpec{ requiredValueCount, applyToBounds } and rewire
  validateOperatorAndValues and collectRequirementBounds to read from
  it. Drop the speculative multi-value error-wording branch in
  validateOperatorAndValues — no operator declares a count > 1, and a
  future multi-value operator can update the wording at the time.
- Extract (*requirementBounds).applyEq as a method so the spec table
  can reference it as (*requirementBounds).applyEq.
- Add TestApplyEq covering first call / second-equal / second-different
  branches, with postconditions that eqVal carries the expected value
  including preservation on the error path.
- Add cross-format consistency rows that exercise Cmp through the
  validator's own logic paths (Eq+Ne and Eq vs upper bound), not
  apimachinery primitives.
- Add TestSupportedPropertyOperatorsRegistryComplete to fail at test
  time if a future entry is added with a non-positive requiredValueCount
  or nil applyToBounds.

Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>

* fix: codespell typo (unparseable -> unparsable)

Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>

---------

Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
@michaelawyu
Copy link
Copy Markdown
Contributor

Thanks, Wei! Yeah, it LGTM ;)

@weng271190436 weng271190436 merged commit 98d372e into Azure:main May 28, 2026
21 of 22 checks passed
@weng271190436 weng271190436 deleted the weiweng/backport20260527 branch May 28, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants